[Bazel] Fix mobile-install for python2 #12540

ThomasCJY · 2020-11-23T20:15:36Z

Background
Fix issue introduced by 1049fe8
It turns out the queue module name is inconsistent across different python versions. We found that:

On some macos system:
- python2 contains both queue and Queue module. All python2 contains Queue module
- python3 only contains queue module
On some Linux system
- python2 contains only Queue module.
- python3 only contains queue module

Therefore, some developers are seeing ImportError: No module named queue errors locally on linux machine after using mobile-install.

Change
Import correct Queue module instead

Test
Local test pass

tools/android/build_incremental_dexmanifest.py

google-cla · 2020-11-23T21:03:26Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

ThomasCJY · 2020-11-23T21:05:58Z

tools/android/build_incremental_dexmanifest.py

@@ -32,7 +32,7 @@

 import hashlib
 import os
-from queue import Queue
+from Queue import Queue


Should we use

try: import Queue from Queue except ImportError: import queue as Queue

here to support both python 2 and 3?

try: from Queue import Queue except ImportError: import queue as Queue

This is the syntax that worked when python 2 is used.

@ThomasCJY can you change the code to the alternative suggested by @arunkumar9t2 ? That'll make this work in both Pyton2 and 3.

updated @arunkumar9t2 @odracirnumira ptal

The change looks reasonable to me. Unfortunately I lack the necessary permissions to stamp the diff. I think @ahumesky is the right person to loop in here?

nkoroste · 2021-02-19T21:39:02Z

Any updates on this? pretty trivial change...

**Background** It turns out the queue module name is inconsistent across different python versions. We found that: * On macos system - python2 contains both queue and Queue module. - python3 only contains queue module * On Linux system - python2 contains only Queue module. - python3 only contains queue module Therefore, some developers are seeing `ImportError: No module named queue` errors locally on linux machine. **Change** Import correct Queue module instead **Test** Local test pass Upstream PR: bazelbuild#12540 --- Automatic squash commit from https://github.sc-corp.net/Snapchat/bazel/pull/83 Cooled by jchen

ahumesky · 2021-03-05T19:10:48Z

I've imported this change internally, but our linters are pretty unhappy about imports "not at top of file". We seem to be doing this in other places so I think I can work around it.

ahumesky · 2021-03-09T02:32:23Z

looks like this is causing the //tools/android:build_incremental_dexmanifest_test test to fail:

tools/android/build_incremental_dexmanifest.py", line 55, in __init__
    self.queue = Queue()
TypeError: 'module' object is not callable

(and also causing 2 internal tests to fail with the same error)

Looks like this is because the original code from queue import Queue imports the Queue class from the module queue, whereas in the except block, the code import queue as Queue imports the module with the name "Queue".

Are these the 3 scenarios?

from queue import Queue (original. Queue class in queue module)
from Queue import Queue (Queue class is is Queue module)
import Queue (import just module, module is callable to produce Queue object)

At least on my Linux machine, scenario 3 doesn't seem to work. There appears to be both queue and Queue in python2, just queue in python3, and the module isn't callable in any of the cases. I think queue in python2 is from a compatibility package:

>>> print(queue.__file__)
/usr/lib/python2.7/dist-packages/queue/__init__.pyc

So unless something is different on macos, seems this change should be:

try:
  # python2 without compatibility package
  from Queue import Queue
except ImportError:
  # python3
  from queue import Queue

ThomasCJY · 2021-03-10T20:21:34Z

@ahumesky yeah you are right. It's a bit confusing but according to the python doc:

python 2 uses Queue module name and the creation of object is Queue.Queue()
https://docs.python.org/2.7/library/queue.html

python 3 uses queue module name and the creation of object is queue.Queue()
https://docs.python.org/3.8/library/queue.html

so the import syntax should be

try:
  # python2 without compatibility package
  from Queue import Queue
except ImportError:
  # python3
  from queue import Queue

ThomasCJY · 2021-03-26T17:36:08Z

@ahumesky is this good to go?

**Background** Fix issue introduced by bazelbuild@1049fe8 It turns out the queue module name is inconsistent across different python versions. We found that: * On some macos system: - python2 contains both queue and Queue module. All python2 contains Queue module - python3 only contains queue module * On some Linux system - python2 contains only Queue module. - python3 only contains queue module Therefore, some developers are seeing `ImportError: No module named queue` errors locally on linux machine after using mobile-install. **Change** Import correct Queue module instead **Test** Local test pass Closes bazelbuild#12540. PiperOrigin-RevId: 369773133

**Background** Fix issue introduced by 1049fe8 It turns out the queue module name is inconsistent across different python versions. We found that: * On some macos system: - python2 contains both queue and Queue module. All python2 contains Queue module - python3 only contains queue module * On some Linux system - python2 contains only Queue module. - python3 only contains queue module Therefore, some developers are seeing `ImportError: No module named queue` errors locally on linux machine after using mobile-install. **Change** Import correct Queue module instead **Test** Local test pass Closes #12540. PiperOrigin-RevId: 369773133

**Background** Fix issue introduced by bazelbuild@1049fe8 It turns out the queue module name is inconsistent across different python versions. We found that: * On some macos system: - python2 contains both queue and Queue module. All python2 contains Queue module - python3 only contains queue module * On some Linux system - python2 contains only Queue module. - python3 only contains queue module Therefore, some developers are seeing `ImportError: No module named queue` errors locally on linux machine after using mobile-install. **Change** Import correct Queue module instead **Test** Local test pass Closes bazelbuild#12540. PiperOrigin-RevId: 369773133

ThomasCJY requested a review from ahumesky as a code owner November 23, 2020 20:15

google-cla bot added the cla: yes label Nov 23, 2020

ThomasCJY commented Nov 23, 2020

View reviewed changes

tools/android/build_incremental_dexmanifest.py Outdated Show resolved Hide resolved

google-cla bot added cla: no and removed cla: yes labels Nov 23, 2020

ThomasCJY force-pushed the jchen-fixMobileInstall branch from a895069 to 56df4ba Compare November 23, 2020 21:05

google-cla bot added cla: yes and removed cla: no labels Nov 23, 2020

ThomasCJY commented Nov 23, 2020

View reviewed changes

jin added the team-Android Issues for Android team label Dec 4, 2020

jin assigned ahumesky Dec 4, 2020

ThomasCJY added 3 commits February 24, 2021 14:51

fix

49d7027

fix

f03bef5

update

ec62e96

ThomasCJY force-pushed the jchen-fixMobileInstall branch from 11d30e7 to ec62e96 Compare February 24, 2021 22:53

update

8d7a5a0

bazel-io closed this in 48eee8b Apr 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bazel] Fix mobile-install for python2 #12540

[Bazel] Fix mobile-install for python2 #12540

ThomasCJY commented Nov 23, 2020 •

edited

Loading

google-cla bot commented Nov 23, 2020

ThomasCJY Nov 23, 2020 •

edited

Loading

arunkumar9t2 Feb 15, 2021

odracirnumira Feb 24, 2021

ThomasCJY Feb 24, 2021 •

edited

Loading

odracirnumira Feb 25, 2021

nkoroste commented Feb 19, 2021

ahumesky commented Mar 5, 2021

ahumesky commented Mar 9, 2021

ThomasCJY commented Mar 10, 2021

ThomasCJY commented Mar 26, 2021

[Bazel] Fix mobile-install for python2 #12540

[Bazel] Fix mobile-install for python2 #12540

Conversation

ThomasCJY commented Nov 23, 2020 • edited Loading

google-cla bot commented Nov 23, 2020

ThomasCJY Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

arunkumar9t2 Feb 15, 2021

Choose a reason for hiding this comment

odracirnumira Feb 24, 2021

Choose a reason for hiding this comment

ThomasCJY Feb 24, 2021 • edited Loading

Choose a reason for hiding this comment

odracirnumira Feb 25, 2021

Choose a reason for hiding this comment

nkoroste commented Feb 19, 2021

ahumesky commented Mar 5, 2021

ahumesky commented Mar 9, 2021

ThomasCJY commented Mar 10, 2021

ThomasCJY commented Mar 26, 2021

ThomasCJY commented Nov 23, 2020 •

edited

Loading

ThomasCJY Nov 23, 2020 •

edited

Loading

ThomasCJY Feb 24, 2021 •

edited

Loading